Skip to content

Repair a node's export policy rules when it registers - #1186

Open
notsrch wants to merge 1 commit into
NetApp:masterfrom
notsrch:fix/1181-node-registration-repair
Open

Repair a node's export policy rules when it registers#1186
notsrch wants to merge 1 commit into
NetApp:masterfrom
notsrch:fix/1181-node-registration-repair

Conversation

@notsrch

@notsrch notsrch commented Sep 4, 2026

Copy link
Copy Markdown

Change description

With autoExportPolicy enabled, the ONTAP NAS drivers write a node's export policy rules in
exactly one place: that node's own ControllerPublishVolume. Nothing ever adds or corrects a
rule afterwards. ReconcileVolumeNodeAccess was a no-op for ontap-nas-economy and
ontap-nas, the periodic node-access loop only reconciles the backend-wide policy, and
Kubernetes does not repeat ControllerPublishVolume while a VolumeAttachment exists. So when a
published node's rule went missing or stale (node IP change, out-of-band edit, or the rule wipe
in #1179), that node failed every mount of the volume until an operator deleted its
VolumeAttachments or edited the policy on the array. Issue #1181 describes two production
cases; the second lasted eight days.

This change adds the missing repair path and triggers it on the one event a rebooted or
re-addressed node always produces: registration.

  1. Drivers. ReconcileVolumeNodeAccess for ontap-nas-economy and ontap-nas now adds
    any missing rules for the given nodes to the volume's own per-qtree or per-volume policy.
    It is add-only: it never removes a rule and never changes which policy is assigned. It skips
    volumes that are not on a Trident-managed per-volume policy (still on the backend policy,
    unpublished, or on a customer-managed policy). Read-only clones on the economy driver share
    their source qtree's policy and are repaired against it; on ontap-nas they have no policy
    of their own and are skipped. The recorded ExportPolicy is trusted rather than re-read
    from ONTAP; a stale record only delays repair to the volume's next publish or unpublish.
  2. Shared helper. The rule-adding half of the existing publish path is extracted into
    ensureNodeAccessRulesForPolicy, which takes a node list, so publish and repair use one code
    path. Callers hold the per-policy lock as before.
  3. Both cores, on AddNode. After registering the node, a background task collects every
    volume that node is published to (a subordinate publication resolves to its share-source
    volume), and calls the driver repair for each one with every node published to that volume.
    A shared RWX volume is therefore repaired for all of its nodes at once, not only for the
    node that registered. Failures are logged at warning and retried at the next registration.
    Only backends that enforce publications take part.
  4. Classic core cleanup. The old per-volume loop inside the synchronous
    reconcileNodeAccessOnBackend is removed. It only ever reached no-op driver
    implementations, so nothing changes for any driver.

What this does not do. The repair is event-driven only; there is no periodic loop. A rule
that goes missing while every node stays up is not repaired until some node published to that
volume re-registers (Trident node pod restart or node reboot). That is a deliberate first step:
a registration is exactly when a node's IP may have changed, and it avoids one ExportRuleList
per published volume per period. A periodic pass on a slow cadence (for example every 10
minutes) can be layered on the same helpers later if wanted. Until then, "restart the node's
Trident pod" is the documented repair action, and the live test below confirms it also clears
the stuck-unpublish case described in #1184.

Fixes #1181.

Project tracking

  • This PR does not require a JIRA ticket (community contribution; details in the issue)

Testing

Summary: unit tests for the helper, both drivers, and both cores; the bug was reproduced on a
live ONTAP cluster on a 26.06.0-based build without this change, and the same test passes with
this change applied. Both live runs were done on builds that also carry the #1179 fix from
PR #1183, so the results reflect the combination, not stock 26.06.0.

Unit tests
  • go build ./..., gofmt -l core storage_drivers/ontap clean. go vet reports one
    pre-existing warning at ontap_nas_qtree.go (WaitGroup.Add in goroutine) that is also on
    master.
  • go test ./core/ ./storage_drivers/ontap/ -count=1 green on the branch. The ontap package
    has one pre-existing flaky test (TestInitializeASANVMe, telemetry goroutine logging after
    the test ends) that fails intermittently on master as well; it is unrelated to this change.
  • New tests:
    • ensureNodeAccessRulesForPolicy (11 cases): multi-node union with de-duplication, CIDR
      filtering, missing policy created then populated, comma-format existing rules not
      recreated, already-exists tolerated, list error tolerated, create and exists errors
      propagated, empty node slice with and without an existing policy.
    • Economy driver ReconcileVolumeNodeAccess (12 cases): happy path multi-node, auto export
      disabled, empty nodes, empty internal name, unset or empty policy name, volume still on
      the backend policy, customer-managed policy, read-only clone on the source policy,
      read-only clone on an unmanaged policy, missing policy recreated, helper error propagated.
    • ontap-nas driver ReconcileVolumeNodeAccess (11 cases): same matrix; read-only clones
      are skipped on this driver.
    • Classic and concurrent core AddNode repairs the export rules of the volumes the node is
      published to (both cores, public API).
    • Repair collection, both cores: repair carries every node published to the volume;
      subordinate publication resolves to the source; source and subordinate produce one repair;
      other nodes' publications are not repaired; unregistered node left out; unknown volume
      skipped; backend without publish enforcement skipped.
    • applyNodeAccessRepairs: errors logged and skipped; stops on cancelled context.
Live reproduction on v26.06.0 + #1179 fix, without this change (fails)

Environment: 3-node RKE2 cluster, Kubernetes v1.35.4, Trident 26.06.0 plus the #1179 fix from
PR #1183, installed by the operator with enableConcurrency: true. One ontap-nas-economy
backend on ONTAP 9.17.1P1 (REST), autoExportPolicy: true, no other volumes on the backend.
The #1179 fix touches no repair path, so this run is representative of stock 26.06.0 for the
behaviour under test. Node IPs below are placeholders.

Setup: one RWX PVC (1 Gi), a DaemonSet mounting it so every node holds a publication and a
VolumeAttachment. ONTAP queried and edited over REST from inside the cluster. Before state:
qtree policy and backend policy each 3 rules (192.0.2.11, 192.0.2.12, 192.0.2.13), 3
publications, 3 VolumeAttachments.

Trigger, from the issue's To Reproduce: delete node-b's rule from the volume's qtree export
policy on the array (DELETE /api/protocols/nfs/export-policies/{id}/rules/{index}, HTTP 200).
Backend policy left intact.

Time Step Observation
T+0 Rule deleted. Qtree policy 2 rules. Established pod on node-b: read and write fail with Permission denied (NFSv3 evaluates the policy per request). Other nodes unaffected.
T+2 min More than three periodic node-access ticks (30 s period). Still 2 rules. Controller log: nothing.
T+2 min New pod on node-b (node already holds a VolumeAttachment, so no ControllerPublish). FailedMount ... error mounting NFS volume <data-lif>:/<flexvol>/<qtree> ... exit status 32, stuck in ContainerCreating. tridentvolumepublications lists all 3 nodes; tridentnodes IPs correct.
T+5 min Fresh ControllerPublish of a different volume on node-b, same backend. Its pod Running. First volume's qtree policy still 2 rules.
T+9 min Trident node pod on node-b restarted. Controller: "Added a new node". Backend policy reconciled (already complete). Qtree policy still 2 rules.
T+11 min Removed every consumer on node-b to force a fresh publish. DaemonSet pod could not finish terminating: NodeUnpublishVolume fails on stat ...: permission denied (#1184). VolumeAttachment held. Force delete did not help; no forced detach within 10 minutes.
T+25 min Rule re-added on the array by hand. Unpublish succeeded; VolumeAttachment released 31 s later; ControllerUnpublish pruned the rule again.
T+26 min Node-b re-admitted. Fresh ControllerPublish rewrote the rule: 3 rules.

Every claim in the issue held: rules are written only by that node's own ControllerPublish for
that volume; the periodic loop and node re-registration do not touch per-qtree policies;
Trident's records stayed correct throughout.

Retest with this change on top of v26.06.0 + #1179 fix (passes)

Same cluster, backend, volume, DaemonSet and query method. Image built from v26.06.0 plus the
#1179 fix plus this branch. Clean state confirmed first: 3 rules in both policies, 3
publications, 3 VolumeAttachments. Controller startup replayed the reconcile for all 10
TridentBackendConfigs with no rule loss.

Time Step Result
T+0 Deleted node-b's rule from the qtree policy. 2 rules; established pod on node-b loses I/O, as before.
T+2 min Four periodic ticks. Still 2 rules. Expected: this change is registration-driven, not periodic.
T+2.5 min Restarted the Trident node pod on node-b. Controller "Added a new node" 2 s later. Qtree policy 3 rules 21 s after registration. Established pod on node-b reads and writes again without a remount. Backend policy and the other volume's policies unchanged.
T+4.5 min Deleted the rule again, then deleted the DaemonSet pod on node-b. Pod stuck in Terminating on the #1184 unpublish failure (40 retries in 35 s), VolumeAttachment held. Same as without this change.
T+5.5 min Restarted the Trident node pod on node-b. Rule restored within seconds of registration. The pending unpublish succeeded on its next retry, the stuck pod cleared, the replacement DaemonSet pod came up Running.

Comparison:

Path Without this change With this change
Periodic 30 s loop no repair no repair (by design, see above)
Publish of another volume on the node no repair not exercised; unchanged code
Node re-registration backend policy only qtree policy repaired within 21 s
Pod stuck on failing unpublish (#1184) only exit: edit the policy on the array node pod restart repairs the rule and the unpublish completes

The 1703945 "Ruleset is in use by a volume" warning on each backend reconcile is still
present on both builds; that is #1180 and out of scope here.

Checklist

  • Unit tests added
  • CHANGELOG updated
  • Documentation: none needed (no user-visible option changes)

AI assistance

Assisted-by: Claude Fable 5 noreply@anthropic.com

With autoExportPolicy enabled, the ONTAP NAS drivers write a node's
export policy rules only in that node's own ControllerPublishVolume.
Nothing adds or corrects a rule afterwards: ReconcileVolumeNodeAccess is
a no-op for ontap-nas-economy and ontap-nas, the periodic node-access
loop touches only the backend-wide policy, and Kubernetes does not
repeat ControllerPublishVolume while a VolumeAttachment exists. A node
whose rule goes missing or stale (IP change, out-of-band edit, or the
rule wipe in NetApp#1179) fails every mount of the volume until an operator
deletes its VolumeAttachments or edits the policy on the array.

This change adds the repair path and runs it when a node registers,
which is the one event a rebooted or re-addressed node always produces.

- Implement ReconcileVolumeNodeAccess for ontap-nas-economy and
  ontap-nas as an add-only repair of the volume's own per-qtree or
  per-volume policy for the given nodes. It never removes a rule or
  changes which policy is assigned, and it skips volumes that are not on
  a Trident-managed per-volume policy. Economy read-only clones are
  repaired against their source qtree's policy; ontap-nas clones have no
  policy of their own and are skipped.
- Extract the rule-adding half of the publish path into
  ensureNodeAccessRulesForPolicy, which takes a node list, so publish
  and repair share one code path.
- In both cores, after AddNode registers the node, a background task
  collects every volume the node is published to (subordinates resolve
  to their share source) and repairs each one for every node published
  to it. Failures are logged and retried at the next registration. Only
  backends that enforce publications take part.
- Remove the classic core's per-volume loop from the synchronous
  reconcile; it only ever reached no-op driver implementations.

The repair is event-driven only. There is no periodic pass, so a rule
lost while every node stays up is repaired at the next registration of
any node published to that volume. A periodic pass can be layered on the
same helpers later.

Verified on a live ONTAP cluster: a qtree rule deleted on the array was
restored 21 s after the node's Trident pod restarted; the periodic loop
alone left it missing.

Fixes NetApp#1181.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant